Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

storage controller: don't hold detached tenants in memory #10264

Merged
merged 13 commits into from
Jan 8, 2025
Merged

storage controller: don't hold detached tenants in memory #10264

merged 13 commits into from
Jan 8, 2025

Conversation

jcsp
Copy link
Contributor

@jcsp jcsp commented Jan 3, 2025

Problem

Typical deployments of neon have some tenants that stay in use continuously, and a background churning population of tenants that are created and then fall idle, and are configured to Detached state. Currently, this churn of short lived tenants results in an ever-increasing memory footprint.

Closes: #9712

Summary of changes

  • At startup, filter to only load shards that don't have Detached policy
  • In process_result, check if a tenant's shards are all Detached and observed=={}, and if so drop them from memory
  • In tenant_location_conf and other tenant mutators, load the tenants' shards on-demand if they are not present

@jcsp jcsp added t/feature Issue type: feature, for new features or requests c/storage/controller Component: Storage Controller labels Jan 3, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 3, 2025 •
edited
Loading

7249 tests run: 6884 passed, 0 failed, 365 skipped (full report)

Flaky tests (1)

Postgres 14

  • test_physical_replication_config_mismatch_max_locks_per_transaction: release-arm64

Code coverage* (full report)

  • functions: 31.2% (8411 of 26979 functions)
  • lines: 47.9% (66779 of 139357 lines)

* collected from Rust tests only

The comment gets automatically updated with the latest test results
22fc95b at 2025-01-08T17:16:13.928Z :recycle:

@jcsp jcsp marked this pull request as ready for review January 3, 2025 13:06
@jcsp jcsp requested a review from a team as a code owner January 3, 2025 13:06
@jcsp jcsp requested review from arpad-m and VladLazar January 3, 2025 13:06
VladLazar
VladLazar reviewed Jan 3, 2025
View reviewed changes
@jcsp jcsp requested a review from VladLazar January 6, 2025 18:22
@jcsp
Copy link
Contributor Author

jcsp commented Jan 6, 2025

Let's get this merged and give it a week in staging -- there's a lot of detach/attach churn there, so we should have an excellent chance of spotting any unforseen issues

VladLazar
VladLazar approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@VladLazar VladLazar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I think attach hook handling is missing a maybe_load_tenant call around service.rs:1603 and that's causing the test failures.

@jcsp
Copy link
Contributor Author

jcsp commented Jan 8, 2025

I think attach hook handling is missing a maybe_load_tenant call around service.rs:1603 and that's causing the test failures.

Yep, thanks for pointing that out.

@jcsp jcsp enabled auto-merge January 8, 2025 13:46
@jcsp jcsp added this pull request to the merge queue Jan 8, 2025
Merged via the queue into main with commit 68d8acf Jan 8, 2025
88 checks passed
@jcsp jcsp deleted the jcsp/drop-detached-tenants branch January 8, 2025 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VladLazar VladLazar VladLazar approved these changes

@arpad-m arpad-m Awaiting requested review from arpad-m arpad-m is a code owner automatically assigned from neondatabase/storage

Assignees
No one assigned
Labels
c/storage/controller Component: Storage Controller t/feature Issue type: feature, for new features or requests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

storage controller: don't hold detached projects in memory
2 participants
@jcsp @VladLazar